Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

string formatting guide #1375

Merged
merged 24 commits into from
Oct 9, 2021
Merged

string formatting guide #1375

merged 24 commits into from
Oct 9, 2021

Conversation

keewis
Copy link
Contributor

@keewis keewis commented Aug 21, 2021

As expressed in #1357, the tutorial does not cover everything on the formats (nor should it!) and that section is pretty difficult to find when trying to look up the available formats (also, I didn't know where to put the register_*_format functions)

Edit: maybe we should add a show_formats function together with the register_*_format functions?

The Quantity formatting is still left because I don't yet know enough about that.

@hgrecco, RTD has a pretty neat PR preview feature which can be enabled through the admin settings. Could you enable this?

@hgrecco
Copy link
Owner

hgrecco commented Aug 21, 2021

@keewis I enabled Build pull requests for this project I guess that is all, right?

@keewis
Copy link
Contributor Author

keewis commented Aug 21, 2021

indeed. You should have gotten a message from github requesting the access. If so, we should be able to close/reopen this to see the effect. Let's try:

@keewis keewis closed this Aug 21, 2021
@keewis keewis reopened this Aug 21, 2021
@hgrecco
Copy link
Owner

hgrecco commented Aug 21, 2021

I don't see it in the status. I am checking if the webhook is ok

@keewis
Copy link
Contributor Author

keewis commented Aug 21, 2021

for reference, here's the docs on that feature: https://docs.readthedocs.io/en/stable/pull-requests.html

@hgrecco
Copy link
Owner

hgrecco commented Aug 21, 2021

Somehow there has not been

@hgrecco hgrecco closed this Aug 21, 2021
@hgrecco hgrecco reopened this Aug 21, 2021
@keewis
Copy link
Contributor Author

keewis commented Aug 21, 2021

it's there now! Thanks!

@hgrecco
Copy link
Owner

hgrecco commented Aug 21, 2021

Just for the record, the read the docs documentation instructs to enable the webhook for Branch or tag creation, Branch or tag deletion and Pushes events It seems that you also need to enable for Pull requests when this feature is used.

@hgrecco
Copy link
Owner

hgrecco commented Aug 21, 2021

@keewis We also need to check that the visibility of the generated docs are fine. ping me to let me know if something is not ok.

@keewis
Copy link
Contributor Author

keewis commented Aug 21, 2021

We also need to check that the visibility of the generated docs are fine.

Not sure if I understand this correctly. Could you elaborate?

Edit: the build currently fails because of an issue with numpy and sparse (it's the same for master)

@hgrecco
Copy link
Owner

hgrecco commented Aug 21, 2021

Ohh, when I was going to link to a reference I found (Privacy levels are only supported on Read the Docs for Business.) So do not worry, I was afraid that the outcome of the preview was only visible to me!

@hgrecco
Copy link
Owner

hgrecco commented Aug 21, 2021

This preview feature is great! How about dividing the index? It seems too crowded. This is a proposal

Getting started
    Installation
    Tutorial

User Guide
    Defining Quantities
    String formatting
    Temperature conversion
    Logarithmic Units

Rolling your own conversions and making a better use of those included (find a short title)
    Defining units
    Different Unit Systems (and default units)
    Contexts

Interaction with other packages
    NumPy Support
    Serialization
    Plotting with Matplotlib
    Pandas support

Other cool stuff (find a title)
    Buckingham Pi Theorem
    Optimizing Performance
    Wrapping and checking functions
    Using Pint for currency conversions
    Using Measurements (I want to extract measurements to another package, and then this will go to Interaction with other packages)

More information
    Command-line script
    Developer reference
    Contributing to Pint
    Frequently asked questions


@keewis
Copy link
Contributor Author

keewis commented Aug 21, 2021

indeed. I think @jules-ch wanted to do a reorganization of the docs and introduce a new theme (see #972), which is where I would do this, too.

@keewis keewis mentioned this pull request Aug 21, 2021
Copy link
Contributor Author

@keewis keewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be ready for a review. I'd especially be interested in the opinions of people learning about string conversion in pint for the first time, since that's the best way for me to find things I wrongly assumed to be common knowledge.

Not sure if you still fit in that category, but: cc @TomNicholas, @jbusecke

Edit: I just noticed that the introduction for unit format specs is not easy to read... I'll fix that.

Comment on lines +45 to +46
"IPython.sphinxext.ipython_directive",
"IPython.sphinxext.ipython_console_highlighting",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like the ipython sphinx extension a lot (I think it's cleaner than the doctest directive). If desired, I can also open a new PR converting our current use of the doctest directive (and code blocks, where appropriate) to that, which might allow us to use pytest to run the doctests in the docstrings.

Comment on lines +46 to +49
u.default_format = "" # TODO: switch to None
ureg.default_format = "" # TODO: switch to None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this PR is merged before #1371, I can do that there

docs/formatting.rst Outdated Show resolved Hide resolved
@keewis
Copy link
Contributor Author

keewis commented Sep 3, 2021

now that the last issue is resolved this should be ready for a final review

@cpascual
Copy link
Contributor

cpascual commented Oct 4, 2021

Hi @keewis , I just read the proposed formatter docs: I found them great!
Just one minor thing: I found that the difference between the C format type specifier for the units and the # modifier for quantities is not easy to grasp on a quick reading (on my first glance I actually assumed that there was a typo and that you meant C where you wrote #). Perhaps a note or some clarification in the text could help?

Maybe something like adding "(see also the related # modifier)" when first mentioning C?

@keewis
Copy link
Contributor Author

keewis commented Oct 4, 2021

they actually do different things, despite their naming: # will convert to a unit that makes the quantity have as few decimal digits as possible while C will reduce the space taken up by operators (e.g. by not padding them with spaces).

I guess we should try to make the naming more distinct (maybe rename C to N for narrow or T for tiny?), and I can also add a more detailed explanation to both, maybe by replacing the format type's name with a "description" column?

Edit: in any case, thanks for the review

@cpascual
Copy link
Contributor

cpascual commented Oct 4, 2021

@keewis you are right! I was still confused even after a not-so-quick read (but the blame is on me, not on the text! )

@keewis
Copy link
Contributor Author

keewis commented Oct 5, 2021

well, texts can be written in a way that is less misleading, even if the reader is less attentive. I will try to see if I can make the difference a bit more visible.

@jules-ch
Copy link
Collaborator

jules-ch commented Oct 9, 2021

@keewis I'm gonna merge this.
It's a good addition to the docs.
If you want to amend this, open a PR I'll be happy to merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

formats, documentation and custom formats
4 participants